Skip to content

feat(metrics): current-in-Qdrant chunk-density snapshot histogram - #1065

Merged
cbcoutinho merged 2 commits into
masterfrom
feat/qdrant-chunk-density-snapshot
Jul 11, 2026
Merged

feat(metrics): current-in-Qdrant chunk-density snapshot histogram#1065
cbcoutinho merged 2 commits into
masterfrom
feat/qdrant-chunk-density-snapshot

Conversation

@cbcoutinho

Copy link
Copy Markdown
Owner

Why

The Astrolabe Cloud — Tenant Fleet dashboard panel "Chunk density distribution (chunks/MB)" renders increase(astrolabe_document_chunk_density_chunks_per_mb_bucket[...]) — an ingest-time flow histogram observed once per document at parse (record_chunk_density). It answers "what was the density of documents as they streamed through ingestion", not "what is the density distribution of the documents currently in Qdrant" (the ingest histogram is monotonic, resets on pod restart, and is window-scoped; deletions/re-ingests are never removed).

This adds a current-state snapshot metric recomputed periodically from live Qdrant contents, so the dashboard can render a faithful current distribution panel next to the ingest-flow one.

Why new data was required

density = chunks / (source_bytes / 1e6). The numerator (total_chunks) is already on every point, but the denominator source_bytes was computed at ingest and then discarded — only file_size (files-only) was persisted, and for text doc types the source size is unrecoverable from Qdrant (only chunk excerpts are stored). So source_bytes must be persisted going forward.

What

  • Persist source_bytes in the Qdrant point payload (payload_keys.SOURCE_BYTES), written in processor._index_document.
  • New GaugeHistogram astrolabe_qdrant_chunk_density_chunks_per_mb_current via a custom collector (first in the repo) — the semantically-correct type for a snapshot distribution that rises and falls. doc_type label; buckets shared with the ingest histogram (CHUNK_DENSITY_BUCKETS) so the two panels are directly comparable.
  • vector_density_snapshot_task (metrics_publisher.py) scrolls chunk_index=0 non-placeholder points on its own slower cadence, computes per-doc_type density, and publishes the snapshot. Scan cap surfaced via astrolabe_qdrant_chunk_density_snapshot_truncated (no silent cap).
  • Forward-only coverage (deliberate): only documents (re)ingested after this ships carry source_bytes; unchanged docs are dedup-skipped on re-scan and stay uncovered. Docs without a usable size are reported via astrolabe_qdrant_chunk_density_uncovered_documents{doc_type} so the coverage gap is explicit rather than silently shrinking the histogram.
  • Config: VECTOR_DENSITY_SNAPSHOT_{ENABLED,INTERVAL(300s),MAX_DOCUMENTS(50000)}; task spawned in both app lifespan branches, gated on the enable flag.

Testing

  • Unittests/unit/test_chunk_density_snapshot_metric.py (bucketing, cumulative le/gcount/gsum exposition, snapshot replace, uncovered/truncated gauges) and tests/unit/vector/test_metrics_publisher.py (compute pagination, truncation, failure-swallow, scroll filter shape, task loop).
  • Integrationtests/integration/test_chunk_density_snapshot.py: compute_chunk_density_snapshot over a real in-memory Qdrant engine (covered vs uncovered vs excluded-placeholder; truncation signalling).
  • Verified end-to-end by driving the real publish_chunk_density_snapshot() against a seeded in-memory Qdrant and scraping the registry: 90 chunks/MB → le=91, 8 chunks/MB → le=10, uncovered file counted separately.

Test coverage follow-ups (per repo CLAUDE.md test gate)

  • Contract (board 11): the Qdrant payload is a cross-repo contract with the astrolabe-cloud-website external document-processor (see payload_keys.py docstring). The processor should write the same source_bytes key — tracked as a follow-up, not in this PR.
  • Dashboard (gitops/Grafana): add the "Current chunk-density distribution (snapshot)" panel querying sum by (le) (astrolabe_qdrant_chunk_density_chunks_per_mb_current_bucket{...}) + an uncovered-docs stat — separate change.

Deck #638 (board 9 — Platform Observability). Shared source_bytes dependency with Deck #636 (density baseline).


This PR was generated with the help of AI, and reviewed by a Human

The Tenant Fleet dashboard panel "Chunk density distribution (chunks/MB)"
renders increase(astrolabe_document_chunk_density_chunks_per_mb_bucket[...]),
an ingest-time flow histogram observed once per document at parse. It does not
represent the density distribution of the documents currently resident in
Qdrant (monotonic, reset-on-restart, window-scoped).

Add a current-state snapshot metric recomputed periodically from live Qdrant
contents:

- Persist source_bytes (ingested_byte_size) in the Qdrant point payload
  (payload_keys.SOURCE_BYTES) — the density denominator, previously computed at
  ingest and then discarded. Numerator (total_chunks) is already on every point.
- New GaugeHistogram astrolabe_qdrant_chunk_density_chunks_per_mb_current
  (custom collector, doc_type label, buckets shared with the ingest histogram
  via CHUNK_DENSITY_BUCKETS) — the correct Prometheus type for a snapshot
  distribution that rises and falls.
- vector_density_snapshot_task scrolls chunk_index=0 non-placeholder points on
  its own slower cadence, computes per-doc_type density, and publishes the
  snapshot. Scan cap surfaced via astrolabe_qdrant_chunk_density_snapshot_truncated
  (no silent cap).
- Forward-only coverage: only documents (re)ingested after this ships carry
  source_bytes. Docs without a usable size are reported via
  astrolabe_qdrant_chunk_density_uncovered_documents{doc_type} so partial
  coverage is explicit rather than silently shrinking the histogram.
- Config knobs VECTOR_DENSITY_SNAPSHOT_{ENABLED,INTERVAL,MAX_DOCUMENTS}; task
  spawned in both app lifespan branches, gated on the enable flag.

Tests: unit (bucketing, cumulative/gcount/gsum exposition, snapshot replace,
uncovered/truncated gauges, compute pagination/truncation/failure-swallow) and
integration (compute over a real in-memory Qdrant engine: covered vs uncovered
vs excluded-placeholder, truncation signalling).

Deck #638 (board 9). Cross-repo payload contract with the astrolabe-cloud-website
external processor to be mirrored as a follow-up (board 11).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @cbcoutinho's task in 3m 2s —— View job


Claude PR Review

  • Gather context (PR body, diff, repo conventions)
  • Review payload_keys.py / processor.py (source_bytes persistence)
  • Review metrics.py (GaugeHistogram collector)
  • Review metrics_publisher.py (snapshot task)
  • Review config.py / app.py wiring
  • Review test coverage
  • Post final review

Note: I wasn't able to execute uv run pytest/ruff/ty in this sandbox (Bash commands beyond simple reads required interactive approval I don't have here), so this is a static read-through of the diff plus a careful trace of the round-1 fix (28a2006). The PR author states 32 unit+integration tests pass locally and ruff/ty are green — I have no reason to doubt that from the code shown, but a maintainer should let CI confirm.


Summary

Clean, well-scoped addition. source_bytes persistence in processor.py/payload_keys.py is minimal and correctly reuses the value already computed for _record_ingest_vector_cost rather than calling ingested_byte_size twice. The _ChunkDensitySnapshotCollector is a sensible use of a custom Prometheus Collector for a GaugeHistogram (rise-and-fall semantics genuinely don't fit a monotonic Histogram), and the round-1 truncation fix in 28a2006 (checking Qdrant's offset is None before the scanned > max_documents comparison) is correct — verified by tracing test_exact_cap_boundary_is_not_truncated and test_truncates_when_corpus_strictly_exceeds_cap against the loop in metrics_publisher.py:compute_chunk_density_snapshot.

Things I checked and consider non-issues (calling out so the reasoning is visible, not because they're findings)

  • GaugeHistogramMetricFamily(..., gsum_value=gsum) (metrics.py) — correct kwarg name for prometheus_client's API; matches the cumulative-bucket assertions in test_chunk_density_snapshot_metric.py.
  • doc_type label cardinality — bounded by the small internal doc-type enum, same pattern already used by the existing document_chunk_density_chunks_per_mb histogram and count_indexed; not a new risk.
  • Custom Collector + REGISTRY.register at module import time (metrics.py:~550) — fine since this service doesn't run under prometheus_client multiprocess mode (start_http_server is used directly, no PROMETHEUS_MULTIPROC_DIR), so there's no per-worker aggregation gap to worry about.
  • bool guard in compute_chunk_density_snapshot (total_chunks/source_bytes isinstance checks excluding bool) — a nice defensive touch since bool is an int subclass; correctly inlined rather than hoisted so ty can narrow the types for the division.
  • Duplicated SOURCE_BYTES on every chunk point rather than just chunk_index=0 — consistent with the existing total_chunks field, which is already duplicated the same way; not a new inefficiency introduced by this PR.
  • Forward-only coverage is well-communicated: uncovered_documents gauge is fully cleared and repopulated each pass (chunk_density_uncovered_documents.clear()) so a doc_type that drops to zero doesn't leave a stale series — confirmed by test_uncovered_gauge_reset_between_snapshots.
  • Failure isolation: publish_chunk_density_snapshot catches broad Exception with a noqa: BLE001 justification consistent with the rest of metrics_publisher.py (a metrics refresh must never disturb ingest) — matches the existing publish_vector_sync_metrics pattern.

Minor / non-blocking observations

  • app.py — the snapshot task startup block is duplicated verbatim between the single-user and multi-user lifespan branches (mirroring the existing vector_sync_metrics_task duplication already in the file), so this isn't new debt introduced by the PR — just noting it's an established pattern here, not something this PR should be asked to refactor.
  • Test coverage is genuinely thorough — the exact-cap-boundary regression test added in the round-1 fix is exactly the right test to pin that behavior, and the integration test against a real in-memory Qdrant engine (placeholder exclusion, chunk_index≠0 exclusion, legacy-doc uncovered path) covers what the mocked unit tests structurally can't prove.
  • Config validators (VECTOR_DENSITY_SNAPSHOT_INTERVAL/MAX_DOCUMENTS gte=1) and the _DEFAULTS/Settings/env-var-mapping triple are all kept in sync — easy to miss one of the three in this codebase's config pattern, but all three were updated correctly here.

Conclusion

No blocking issues found. The round-1 SonarCloud Reliability fix (switching to pytest.approx for float count comparisons) and the truncation-boundary fix both look correct and are covered by regression tests. Good to merge from a code-review standpoint, contingent on CI (ruff/ty/tests/SonarCloud gate) passing green.

Round-1 review + SonarCloud follow-ups on the chunk-density snapshot.

- compute_chunk_density_snapshot: check Qdrant's authoritative end-of-scroll
  (offset is None) FIRST and only flag truncated when scanned strictly exceeds
  the cap with more pages pending. Qdrant returns a non-None next offset even at
  the exact end, so the previous `scanned >= cap and offset is not None` could
  fire the alertable ..._snapshot_truncated gauge on a collection sized exactly
  at the cap. Tolerates one page of slop; adds an exact-boundary regression test
  and reshapes the truncation test to the corrected semantics.
- Tests: use pytest.approx for the metric-count assertions (SonarCloud flagged
  exact float-equality as a reliability bug) and drop an async-def-without-await
  in the snapshot-task test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cbcoutinho

Copy link
Copy Markdown
Owner Author

Round-1 findings addressed in 28a2006e:

  • 🟡 False-positive truncated at exact scan-cap boundary: fixed. compute_chunk_density_snapshot now checks Qdrant's authoritative end-of-scroll (offset is None) first and only sets truncated when scanned strictly exceeds the cap with more pages pending — so a collection sized exactly at the cap (where Qdrant still returns a non-None next offset) no longer trips the alertable gauge. Tolerates one page of slop, per your first suggestion. Added an exact-boundary regression test and reshaped the truncation test to the corrected semantics.

Also cleared the SonarCloud Quality Gate failure (C Reliability on new code):

  • Switched the metric-count assertions in test_chunk_density_snapshot_metric.py to pytest.approx (Sonar flagged exact float-equality as a reliability bug — the values are integer counts stored as floats).
  • Dropped an async def without await in the snapshot-task test (minor smell).

32 unit + integration tests pass; ruff + ty green locally.

@sonarqubecloud

Copy link
Copy Markdown

@cbcoutinho
cbcoutinho merged commit c73cd4e into master Jul 11, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant